Skip to content

AVRO-2825: [csharp] Resolve: C# Logical Types throw exception on unknown#3727

Merged
RyanSkraba merged 10 commits intoapache:mainfrom
RawScape:fix-csharp-logical-types
May 10, 2026
Merged

AVRO-2825: [csharp] Resolve: C# Logical Types throw exception on unknown#3727
RyanSkraba merged 10 commits intoapache:mainfrom
RawScape:fix-csharp-logical-types

Conversation

@RawScape
Copy link
Copy Markdown
Contributor

@RawScape RawScape commented Apr 8, 2026

What is the purpose of the change

This pull request resolves AVRO-2825 where the C# implementation threw a SchemaParseException or AvroTypeException when encountering unrecognized Logical Types. This behavior was inconsistent with the Avro specification and the Java implementation, which should fall back to the underlying base type.

This is a takeover of PR #2751. In addition to bringing the code up to date with the current main branch, this PR addresses the following:

  • Core Parser Fix: Resolved a bug in LogicalSchema.NewInstance where logical types applied to flattened complex types (like enums or records) caused a crash because the base type name was passed to the parser without its required properties (symbols, fields, etc.).
  • Review Feedback: Addressed all open comments from the original PR regarding syntax (trailing commas) and logic (incorrect use of nullable byte arrays).

Verifying this change

This change added and refactored tests and can be verified as follows:

  • Refactored AvroGenSchemaTests.cs: Renamed NotSupportedSchema to UnknownLogicalTypesFallbackToBaseType and updated assertions to verify that both primitive and complex schemas with unknown logical types now result in successful code generation (exit code 0).
  • Updated UnknownLogicalTypeTests.cs: Corrected TestGetCSharpType_IsFalse to handle reference types (string, byte[]) properly, ensuring tests no longer fail due to the lack of runtime distinction for nullable reference types.
  • Manually verified that the fix in LogicalSchema.cs correctly clones the JSON token and strips the logicalType property to allow recursive parsing of complex base schemas without infinite loops.

Documentation

  • Does this pull request introduce a new feature? No
  • If yes, how is the feature documented? Not applicable

@github-actions github-actions Bot added the C# label Apr 8, 2026
Comment thread lang/csharp/src/apache/test/Schema/SchemaTests.cs Fixed
@RawScape
Copy link
Copy Markdown
Contributor Author

RawScape commented Apr 16, 2026

@zcsizmadia @KalleOlaviNiemitalo I have pushed a fix for the CodeQL warning last week.
Please let me know if there is anything else expected from me at this stage.

"profiles": {
"Avro.codegen": {
"commandName": "Project",
"commandLineArgs": " -s C:\\Users\\Thomas.Bruns\\source\\repos\\AzureDevOps\\TQL.Kafka.Samples\\TQL.Kafka.Samples\\TQL.Kafka.Samples.Messages\\v1\\cdc_tql_dbo_tbldrops.avsc .\\tab --namespace NO_SCHEMA_NAMESPACE:TQL.DEMO"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might have been a mistakenly added file 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, @RyanSkraba! I inherited that file when I brought Tom's original branch over.
I have deleted it and pushed the update.

@RyanSkraba RyanSkraba added the branch-1.12 (temporary) To prioritize for 1.12.2 label May 10, 2026
@RyanSkraba
Copy link
Copy Markdown
Contributor

Thanks for the fixes, I've gone through the comments in #2751 and it looks like they've all been addressed! Great work carrying this forward. Thanks for keeping @TomBruns' authorship present, all contributors count 😄

@RyanSkraba RyanSkraba merged commit c5f66c7 into apache:main May 10, 2026
8 checks passed
RyanSkraba pushed a commit that referenced this pull request May 10, 2026
…own (#3727)

* AVRO-2825: [csharp] Resolve: C# Logical Types throw exception on unknown logical type

(cherry picked from commit ad61af0)

* [AVRO-3941] CSharp Resolve missing namespace issue

(cherry picked from commit e5a7ded)

* Resolve requested changes on PR 2751

(cherry picked from commit 8909529)

* uncomment testcase after testing

(cherry picked from commit 235d329)

* savepoint remove local setting files

(cherry picked from commit f7b4872)

* backout chags for a different pr, adjust unit test

(cherry picked from commit 60463b5)

* changes from code review

(cherry picked from commit 42b0fbd)

* Resolve remaining feedback for AVRO-2825 and fix complex logical type parsing

* Fix CodeQL warning: Use pattern matching to assert logicalSchema type safely

* Remove accidentally tracked launchSettings.json

---------

Co-authored-by: Tom Bruns <TBruns@tql.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch-1.12 (temporary) To prioritize for 1.12.2 C#

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants